Skip to content

Use new param API in ext/pcntl #7751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 21, 2021
Merged

Conversation

arnaud-lb
Copy link
Member

No description provided.

@arnaud-lb arnaud-lb marked this pull request as ready for review December 10, 2021 17:17
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@Girgias
Copy link
Member

Girgias commented Dec 10, 2021

Just to comment, but as far as I know the "old" ZPP API is not deprecated>
FastZPP inlines the logic for handling params within the function which increases binary size (although it might be negligible), however, union parameter types are only supported through FastZPP.

@arnaud-lb
Copy link
Member Author

Oh ok, I believed it was. Thank you clarifying that.

Is there a recommended practice on choosing between FastZPP and ZPP ?

@Girgias
Copy link
Member

Girgias commented Dec 10, 2021

Oh ok, I believed it was. Thank you clarifying that.

Is there a recommended practice on choosing between FastZPP and ZPP ?

Not that I know of, FastZPP is usually used when the function is common, as it can speed up it's execution significantly as ZPP cannot be inlined due to the variable arguments.
Or when union types are involved as those are only supported through FastZPP.

It might make sense to convert everything to fast ZPP but I have no idea if it is that useful :-/

@cmb69
Copy link
Member

cmb69 commented Dec 10, 2021

When the fast ZPP has been introduced, I was very reluctant to apply it everywhere (e.g. I applied it only to a few ext/gd functions), but it seems to me that has been applied to so many functions where the performance advantage is not even measurable, so that it is okay to use it everywhere. After all, readability is much better.

@krakjoe krakjoe merged commit aa35499 into php:master Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants